Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix!: ResponseMessageをResponseStatusに名称変更 #5399

Open
wants to merge 12 commits into
base: master
Choose a base branch
from

Conversation

Qs-F
Copy link
Contributor

@Qs-F Qs-F commented Feb 19, 2025

関連URL

https://kufuinc.slack.com/archives/CGC58MW01/p1739929345355399

概要

ResponseMessageTypeをルートからexportするようにした

変更内容

  • ResponseMessageTypeをルートからexportするようにした

確認方法

@Qs-F Qs-F marked this pull request as ready for review February 19, 2025 01:52
@Qs-F Qs-F requested a review from a team as a code owner February 19, 2025 01:52
@Qs-F Qs-F requested review from AtsushiM and s-sasaki-0529 and removed request for a team February 19, 2025 01:52
Copy link

pkg-pr-new bot commented Feb 19, 2025

Open in Stackblitz

npm i https://pkg.pr.new/kufu/smarthr-ui@5399

commit: e764baf

@Qs-F Qs-F marked this pull request as draft February 21, 2025 09:20
@Qs-F Qs-F changed the title fix: ResponseMessageType型をexportするように修正 fix!: Mar 3, 2025
@Qs-F Qs-F changed the title fix!: fix!: ResponseMessageをResponseStatusに名称変更 Mar 3, 2025
@Qs-F Qs-F marked this pull request as ready for review March 3, 2025 10:27
@Qs-F
Copy link
Contributor Author

Qs-F commented Mar 3, 2025

あ〜 exportsフィールドを追加するとlibとかからimportしてたやつが使えないのか… そりゃそうだ

@Qs-F Qs-F requested a review from AtsushiM March 5, 2025 06:32
@Qs-F
Copy link
Contributor Author

Qs-F commented Mar 5, 2025

libディレクトリからもimportできるように調整完了

Copy link
Collaborator

@uknmr uknmr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ローカルで確認して抜け漏れなさそうなことを確認しました。
export 周りはうっかり挙動を変えないように何らかの形でテストを書けるといいのかしら(パッと書き方は思い浮かばず)。
また、どこの部分が変わったことによる Breaking change なのかは PR の description あたりにまとまっていると後で(リリースノート書くときなど)に楽そうな気がしました。

@@ -7,7 +7,6 @@ import { Stack } from '../../../Layout'
import { Text } from '../../../Text'
import { MultiComboBox } from '../MultiComboBox'

// eslint-disable-next-line storybook/prefer-pascal-case
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

q: 不要そう?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

そもそもstorybookってlintの対象外になってませんか?
対象になっていた時代の名残なような気がします

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

おっ、そうなんですね? 対象外であれば消しで問題ないです!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ですね!ここは消して良さそうだったので消しました

Comment on lines -1 to -3
/* eslint-disable smarthr/a11y-input-has-name-attribute */
/* eslint-disable smarthr/a11y-prohibit-input-placeholder */
/* eslint-disable smarthr/a11y-input-in-form-control */
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

q: 意図的?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

消されててエラーなかった気がしたのでそのままにしちゃったんですが、一応確認してきます!

@@ -1,4 +1,3 @@
/* eslint-disable smarthr/a11y-input-in-form-control */
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

q: 意図的?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

消されててエラーなかった気がしたのでそのままにしちゃったんですが、一応確認してきます!

"./src/*"
]
},
"target": "ES2017"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

q: 意図的?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

うお、これは意図的じゃないです!!フォーマットかかっただけかと思ってたら見落としてました 😇

Copy link
Member

@AtsushiM AtsushiM left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

修正自体は良さそう。
uさん指摘箇所のご確認お願いします

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants